feat: introduce brand presence semrush stats api#2823
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
This PR will trigger a patch release when merged. |
vivesing
changed the title
MysticatBot
left a comment
MysticatBot
left a comment
There was a problem hiding this comment.
Hey @vivesing,
Verdict: Request changes - capability mismatch and UTC date math issue need fixing before merge.
Complexity: HIGH - large diff; API surface + FACS capability change.
Changes: Adds Elements-backed brand presence stats endpoint with optional weekly trends, mirroring the existing Postgres-RPC handler (14 files).
Note: CI checks are currently failing (validate-pr-title) - resolve before merge.
Must fix before merge
- [Important] Capability mismatch:
organization:readvsbrand:read-src/routes/required-capabilities.js:313(details inline) - [Important]
defaultStatsDateRange()uses local-time arithmetic instead of UTC -src/controllers/elements.js:137(details inline) - [Important] Missing OpenAPI spec - CLAUDE.md rule: "Define API contract in
docs/openapi/before implementation" - [Important] Missing integration tests - CLAUDE.md rule: "New or modified endpoints must include integration tests in
test/it/" - [Important] Empty
projectIdssilently produces unscoped Semrush queries when a brand has no projects -src/controllers/elements.js:590
Non-blocking (5): minor issues and suggestions
- nit: PR title
[WIP]prefix fails conventional-commit validation - remove when ready to merge - suggestion: Add a code comment explaining why TotalExecutions omits the project filter in aggregate mode while siblings include it -
src/support/elements/elements-service.js:370 - suggestion: Guard against inverted date range when only
endDateis user-supplied and falls before the computed defaultstartDate-src/controllers/elements.js:600 - suggestion: Consider
MAX_RANGE_DAYSvalidation consistent with sister routes -src/controllers/elements.js:800 - suggestion:
transformStatsSimpleNumericResponsereturning 0 for missing/malformed upstream data masks failures silently; consider logging a warning or returning null -src/support/elements/definitions/brand-presence-stats.js:50
MysticatBot
added
ai-reviewed
|
Mysticat review failed: Claude CLI crashed (exit 1): stderr= stdout={"type":"result","subtype":"error_max_budget_usd","duration_ms":58158,"duration_api_ms":1739571,"is_error":true,"num_turns":1,"stop_reason":null,"session_id":"1dc35828-7a6c-41dc-9429-6d82b9ea1143","total_cost_usd":10.026556300000001,"usage":{"input_tokens":0,"cache_creation_input_tokens":0,"cache_re |
# Conflicts: # docs/index.html
The merge commit's conflict resolution took main's stale docs/index.html verbatim; rebuild from the fully-merged OpenAPI sources so it reflects both main's changes and the new brand-presence/stats endpoint.
|
Review Comments Addressed #: 1 |
There was a problem hiding this comment.
Hey @vivesing,
Verdict: Request changes - date range validation gap and missing integration tests need addressing.
Complexity: HIGH - large diff; API surface + FACS capability change.
Changes: Adds Elements-backed brand presence stats endpoint with optional weekly trends, mirroring the existing Postgres-RPC handler (19 files).
Note: Recommend a human read before merge - this change adds a new OpenAPI contract (shared-contract change). The bot review is a complement to, not a replacement for, a human read here.
Note: CI checks are currently failing (branch-deploy, deploy-stage) - resolve before merge.
Must fix before merge
- [Important] Missing
MAX_RANGE_DAYSvalidation - sister endpoints (listOwnedUrls,listDomainUrls) enforce a 366-day cap viaMAX_RANGE_DAYS; this endpoint allows unbounded date ranges to Semrush, creating a resource-exhaustion vector inconsistent with the established pattern -src/controllers/elements.js(getStats handler, after date validation). Fix: add the same 5-lineMAX_RANGE_DAYScheck from the sister handlers. - [Important] Missing integration tests - CLAUDE.md rule: "New or modified endpoints must include integration tests in
test/it/". This was flagged in the prior review and remains unresolved. Add a test file undertest/it/postgres/following the existingbrand-presence-topic-promptspattern; cover at minimum: happy-path aggregate, single-region, brand-with-no-projects 404.
Non-blocking (4): minor issues and suggestions
- suggestion: Guard against inverted date range when only
endDateis user-supplied and falls before the computed defaultstartDate(e.g.endDate=2025-01-01with nostartDateproducesstartDate > endDatesilently). Add a post-merge validation after defaults are applied -src/controllers/elements.js - suggestion:
transformStatsSimpleNumericResponsereturning 0 for missing/malformed upstream data masks failures silently; consider logging a warning so production anomalies surface in observability -src/support/elements/definitions/brand-presence-stats.js:46 - suggestion: Add a code comment explaining why Visibility wraps
CBF_modelin{ op: 'or', filters: [...] }while Mentions/Citations use a bare{ op: 'eq' }- the shapes intentionally match Semrush's observed behavior per the design doc, and the asymmetry will confuse a future reader -src/support/elements/definitions/brand-presence-stats.js:113 - suggestion: Consider a per-call timeout or increasing
STATS_TRENDS_WEEK_CONCURRENCYto reduce worst-case endpoint latency (36 upstream calls with concurrency=4 can approach Lambda timeout under intermittent Semrush slowness) -src/support/elements/elements-service.js
Previously flagged, now resolved
- Capability mismatch fixed to
brand:read(consistent with sister brand-presence routes) - UTC date math now uses
addDaysToDateanchored to T12:00:00Z - OpenAPI spec added (
docs/openapi/serenity-api.yaml) with full endpoint definition and contract test - Empty
projectIdsnow guarded with explicit 404 ("No Semrush projects configured for brand")
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 4m 20s | Cost: $7.68 | Commit: f709d77000011c6578ce89ad533f00329bf9e5cc
If this code review was useful, please react with 👍. Otherwise, react with 👎.
MysticatBot
added
the
needs-human-review
… gap Enforce MAX_RANGE_DAYS=56 (8 weeks) on the effective startDate/endDate range, matching the Brand Presence date picker's max selection and the trends fan-out cap, mirroring the MAX_RANGE_DAYS pattern already used by listOwnedUrls/listDomainUrls. Also documents (in the plan doc) why this endpoint - and every ElementsController endpoint - has no integration test coverage: the IT docker-compose stack only mocks the classic Semrush Project Engine API, not the newer Elements API this controller depends on. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Hey @vivesing,
Verdict: Approve - both prior blocking findings resolved; MAX_RANGE_DAYS validation and element ID alignment are well implemented.
Complexity: HIGH - large diff; API surface + FACS capability change.
Changes: Adds Elements-backed brand presence stats endpoint with optional weekly trends, mirroring the existing Postgres-RPC handler (20 files).
Note: Recommend a human read before merge - this change adds a new OpenAPI contract (shared-contract change). The bot review is a complement to, not a replacement for, a human read here.
Note: CI integration-test job (it-postgres) is still pending - verify it passes before merge.
Non-blocking (3): minor issues and suggestions
- suggestion: Guard against inverted date range when only
endDateis user-supplied and falls before the computed defaultstartDate(e.g.endDate=2025-01-01with nostartDateproducesstartDate > endDateafter defaults are applied but before the string comparison catches it, since the existing check runs on the raw inputs before defaulting) -src/controllers/elements.js - suggestion: Consider extracting
MAX_RANGE_DAYS = 56to a named constant alongsideSTATS_TRENDS_WEEK_CONCURRENCYin a shared constants location, so the UI date-picker cap and this server-side cap stay discoverable together -src/controllers/elements.js - nit:
buildStatsTotalExecutionsPayloadwrapsCBF_ws_brandandCBF_modeleach in{ op: 'or', filters: [...] }(single-item OR lists), whilebuildStatsMentionsPayloaduses bare{ op: 'eq' }for both. The asymmetry is intentional (matching Semrush's observed element behavior per the plan doc) but a one-line comment noting "shape mirrors confirmed Semrush sample payload" would prevent future readers from "fixing" it -src/support/elements/definitions/brand-presence-stats.js
Previously flagged, now resolved
- MAX_RANGE_DAYS validation added (56-day cap, UTC Date.parse arithmetic, boundary tests included)
- TOTAL_EXECUTIONS element aligned to same CBF_ws_brand/CBF_model/CBF_project filter shape as Mentions/Visibility/Citations
- Integration tests acknowledged as pre-existing infra gap (no Elements mock container exists for any ElementsController endpoint); follow-up tracked separately
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 0m 3s | Cost: $5.39 | Commit: 43f8e7b4f517e16b0a4790f843c3445a04173798
If this code review was useful, please react with 👍. Otherwise, react with 👎.
# [1.657.0](v1.656.0...v1.657.0) (2026-07-16) ### Features * introduce brand presence semrush stats api ([#2823](#2823)) ([be416a8](be416a8))
|
🎉 This PR is included in version 1.657.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
…2827) ### New endpoint: `GET /v2/orgs/:spaceCatId/brands/:brandId/serenity/brand-presence/market-tracking-trends` ## Summary Adds a Semrush **Elements**-backed endpoint returning **weekly per-competitor mentions + citations** for the tracked brand and its competitors — the data behind the **Competitor Comparison** chart (`CitationsMentionsTrendChart`) on the `brand-presence-sr-ui` dashboard. Full design notes, the confirmed Semrush payload/response shapes, and resolved decisions are in `docs/elements/market-tracking-trends-plan.md` (modeled on `brand-presence-stats-plan.md` from #2823). All brand names and metrics in the docs/tests are synthetic. > The element mapping was verified by observing the Brand Presence MFE's "Market Tracking" chart request/response shapes (Mentions and Citations tabs) — not inferred from the migration wiki, whose documented citations trend element (`b81af644`, domain-level) is *not* what this chart uses. ## Element mapping Both are `type: "line"`, `auto_bucketing: "week"`, **no brand filter** (so competitors come back natively as tracked-benchmark `legend`s): | Chart metric | Element | Constant | Project filter col | Value field | |---|---|---|---|---| | Mentions | `b5281393` | `TRENDS_MV` (existing) | `CBF_project` (singular) | `y__mentions` | | Citations | `2e5a6f4e` | `MARKET_CITATIONS_TREND` (**new**) | `CBF_projects` (**plural**) | `y__mentions` |⚠️ Both elements expose the value under the key `y__mentions`; in the citations element that number is the **citation** count (Semrush reuses the generic key). The transform maps each to the right metric. ## What changed - **Route + auth scaffold** (`routes/index.js`, `routes/facs-capabilities.js` → `llmo/can_view`, `routes/required-capabilities.js` → `brand:read`): registers the route under the existing brand-scoped `authorizeOrg` pattern; `brandId` reuses the existing LLMO FACS brand alias. - **Controller** `getMarketTrackingTrends` (`controllers/elements.js`): mirrors the URL-inspector handlers — `authorizeOrg`, `siteId` cross-check, optional dates with a 28-day trailing default + validation, region→`projectId` resolution (else aggregate across all the brand's projects), `cachedOk`. - **Definitions** (`support/elements/definitions/market-tracking-trends.js`, new): weekly payload builders (`CBF_project` vs `CBF_projects`, no brand filter, no `comparison_data_formatting`) + `transformMarketTrackingTrends` — groups both `blocks.lines[]` by ISO week, splits the brand line from competitor lines, defaults a missing metric to 0. - **Service** `getMarketTrackingTrends` (`support/elements/elements-service.js`): fetches the two elements in parallel; no per-week fan-out (elements are pre-bucketed) and no per-competitor fan-out (`legend` carries them). `projectId` (single region) takes precedence over `projectIds` (aggregate). - **Element id** (`support/elements/element-ids.js`): new `MARKET_CITATIONS_TREND`. - **Design doc** (`docs/elements/market-tracking-trends-plan.md`, new). ## Response shape ```json { "weeklyTrends": [ { "week": "2026-07-05", "weekNumber": 27, "year": 2026, "mentions": 900, "citations": 5000, "competitors": [ { "name": "Rival One", "mentions": 150, "citations": 300 } ] } ] } ``` ## Testing - New/added unit suites: definitions (payload builders + transform), service (`getMarketTrackingTrends`), controller (auth, date defaulting/validation, region resolution + 404, `siteId` cross-check, error mapping), and route registration. - Route-param FACS classification + required-capabilities drift suites pass. - Weekly-only bucketing for now; visibility intentionally excluded (out of scope for this chart). ## Out of scope UI wiring (DAL service + hook + `SRCompetitorComparisonSection`); Competitor Summary / Share of Voice / Sentiment endpoints; day/month bucketing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: hjen_adobe <hjen@adobe.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3 participants
New endpoint: GET /v2/orgs/:spaceCatId/brands/:brandId/serenity/brand-presence/stats
Summary
Implements
GET /v2/orgs/:spaceCatId/brands/:brandId/serenity/brand-presence/stats— the Elements (Semrush)-backed equivalent of the existing Postgres-RPCbrand-presence/statsendpoint (llmo-brand-presence.js#createBrandPresenceStatsHandler). Returns aggregated KPI stats (total_executions,average_visibility_score,total_mentions,total_citations) for a brand over a date range, with an optional weeklytrends[]breakdown.Full design notes, confirmed Semrush payload/response shapes, and resolved open decisions are documented in
docs/elements/brand-presence-stats-plan.md.What changed
src/controllers/elements.js,src/routes/index.js,src/routes/facs-capabilities.js,src/routes/required-capabilities.js): registers the new route under the existing brand-scopedauthorizeOrgauth pattern (llmo/can_viewFACS capability,organization:readS2S capability).getStatswired to real Semrush aggregation (src/controllers/elements.js):startDate/endDate(28-day trailing window when omitted).regionCode/region_code/regionto a single SemrushprojectId(viaresolveRegionProjectId) when a specific region+language is selected — the common case, no fan-out needed.siteId/site_idbelongs to the requested brand.cachedOk.src/support/elements/definitions/brand-presence-stats.js): payload builders + response transformers for the 4 backing Semrush elements —TOTAL_EXECUTIONS,MENTIONS,VISIBILITY,CITATIONS_KPI— including the confirmed quirks:CBF_brand+CBF_projects(plural), while Mentions/Visibility useCBF_ws_brand+CBF_project(singular).comparison_start_date/comparison_end_date/comparison_data_formattingare intentionally omitted — the/statscontract has no period-over-period comparison field.getBrandPresenceStats(src/support/elements/elements-service.js): fetches the 4 KPI elements in parallel for the requested range, and — whenshowTrends=true— re-runs the same 4 calls once per week (bounded concurrency) to populatetrends[], since no single Semrush element returns all 4 metrics pre-bucketed by week.src/support/elements/week-utils.js):addDaysToDate/splitDateRangeIntoWeeksBackward, copied verbance.jsso trend week boundaries (backward fromendDate`, max 8 weeks) match the reference handler exactly.docs/elements/brand-presence-stats-plan.md, new): full API reference (params, response shape, errortable), Semrush element mapping with confirmed and the resolved design decisions(region/project scoping,
total_executionsaggregation, visibility units, brand-filter retention).Notable decisions / follow-ups
has_data_for_last_weekfield — an earlier assumption (carried from wiki docs) that turned out not to exist in the real reference contract.categoryId(s),topicIds,origin,userIntent,promptBrandingare accepted but currently no-ops — no confirmedSemrush Elements filter equivalent yet.
project_idonTOTAL_EXECUTIONSscopes correctly to just the a units spot-check on theaverage_visibility_score×100 conversion.Test plan
npm test— 15034 passing, 0 failing; `eved (98.64%→99.76% stmts, 92.53%→94.04% branches)npm run lint— cleantest/support/elements/definitions/brand-presence-stats.test.js,test/support/elements/week-utils.test.jstest/support/elements/elements-service.test.js,test/controllers/elements.test.js